Skip to content

Conversation

abebus
Copy link
Contributor

@abebus abebus commented Oct 10, 2025

In Tools/build/deepfreeze.py nsmallposints was still 256.

Although deepfreeze is no longer used (#116919), this script still needs to be kept and be actual until it is completely removed.

This PR steals the parser of those numbers from Tools/build/generate_global_objects.py and puts it in a new script, so getting constants can be unified.

@bedevere-app
Copy link

bedevere-app bot commented Oct 10, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@Eclips4 Eclips4 changed the title gh-133160: fix Tools/build/deepfreeze.py nsmallposints gh-133059: fix Tools/build/deepfreeze.py nsmallposints Oct 10, 2025
@Eclips4 Eclips4 self-requested a review October 10, 2025 15:28
@abebus abebus marked this pull request as ready for review October 12, 2025 12:33
import os

SCRIPT_NAME = 'Tools/build/consts_getter.py'
__file__ = os.path.abspath(__file__)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you redefine __file__?


SCRIPT_NAME = 'Tools/build/consts_getter.py'
__file__ = os.path.abspath(__file__)
ROOT = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about builds that are performed in a different folder? It is possible to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these constants were stolen from Tools/build/generate_global_objects.py, so in this case I think it will be more than enough not to override __file__ and just specify a valid path to Include's

@@ -0,0 +1,22 @@
import os

SCRIPT_NAME = 'Tools/build/consts_getter.py'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, I think it's not. it's only used in scripts that generate .c files to put a comment on top to indicate that those file were generated by this particular script

will remove

@abebus abebus requested a review from sobolevn October 13, 2025 17:50
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one minor comment, otherwise LGTM.

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Copy link
Member

@efimov-mikhail efimov-mikhail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Eclips4 Eclips4 merged commit 999ab89 into python:main Oct 17, 2025
55 checks passed
@Eclips4
Copy link
Member

Eclips4 commented Oct 17, 2025

Thank you Albert!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants